fix: cover select all edge cases #10187
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
this changes the select all logic. previously the select all prompt would appear when selecting more than the page limit's worth of objects across the list. if the selected count equaled the total rows in the result set, the clear selection prompt would appear. now, the select all prompt appears when the selection includes all visible rows in the result set, but the selection is not the entire result set. if the selection otherwise exceeds the page limit, we show the clear selection prompt. this fixes the select all prompt appearing when selecting single rows across pages, as well as the select all prompt not appearing when selecting all rows on the last page of results that does not hit the page limit. there was also an issue where, for searches and experiments, the select all prompt would appear when selecting 20 rows regardless of the page size, which is also fixed here.
✅ Deploy Preview for determined-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
EmilyBonar
suggested changes
Nov 5, 2024
There was a problem hiding this comment.
Actually, this breaks the Deselect step in the experimentSpec beforeEach, which then breaks some of the tests. Here's the function that isn't working:
await test.step('Deselect', async () => {
const count = await getCount();
if (count !== 0) {
await grid.headRow.clickSelectHeader();
const isClearSelectionVisible =
await projectDetailsPage.f_experimentList.tableActionBar.clearSelection.pwLocator.isVisible();
if (isClearSelectionVisible) {
await projectDetailsPage.f_experimentList.tableActionBar.clearSelection.pwLocator.click();
}
}
});
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10187 +/- ##
==========================================
- Coverage 54.32% 50.86% -3.47%
==========================================
Files 1259 953 -306
Lines 157303 130328 -26975
Branches 3644 3651 +7
==========================================
- Hits 85462 66287 -19175
+ Misses 71708 63908 -7800
Partials 133 133
Flags with carried forward coverage won't be shown. Click here to find out more.
|
JComins000
approved these changes
Nov 5, 2024
github-actions bot
pushed a commit
that referenced
this pull request
Nov 5, 2024
(cherry picked from commit 11a2581)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ticket
ET-844
Description
this changes the select all logic. previously the select all prompt would appear when selecting more than the page limit's worth of objects across the result set. if the selected count equaled the total rows in the result set, the clear selection prompt would appear. now, the select all prompt appears when the selection includes all visible rows in the result set, but the selection is not the entire result set. if the selection otherwise exceeds the page limit, we show the clear selection prompt.
this fixes the select all prompt appearing when selecting single rows across pages, as well as the select all prompt not appearing when selecting all rows on the last page of results that does not hit the page limit. there was also an issue where, for searches and experiments, the select all prompt would appear when selecting 20 rows regardless of the page size, which is also fixed here.
Test Plan
when visiting the new experiment list, searches page, or runs page, when selecting all rows on the current page, the select all prompt should appear. after that, all rows in the table should appear selected. when deselecting a row, the clear selection prompt should appear. if you then navigate to the next page, the select all prompt should appear again.
Checklist
docs/release-notes/See Release Note for details.